Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature | Reference Homepage redirect #708

Merged
merged 3 commits into from
May 8, 2023

Conversation

ThatcherK
Copy link
Contributor

Ticket #519

@ThatcherK ThatcherK requested a review from akmiller01 May 5, 2023 07:53

if domain == 'reference.iatistandard.org':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to check here for both domain and the path. Because I interpreted the ticket to only redirect reference.iatistandard.org and reference.iatistandard.org/ exclusively. I think catching this domain here would also redirect reference.iatistandard.org/anything-else which I don't think we want.

@@ -86,6 +90,8 @@ def redirected_url(self):
if self.stripped_path.startswith(self.wildcard_redirect_urls):
redirect_match = next(dict_value for dict_key, dict_value in settings.REFERENCE_NAMESPACE_WILDCARD_REDIRECT_DICT.items() if self.stripped_path.startswith(dict_key))
self.stripped_path = self.remove_index_html(self.stripped_path)
print('wowowowowo')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might have left debugging messages in the code here :)

@ThatcherK ThatcherK requested a review from akmiller01 May 5, 2023 14:38

if domain == 'reference.iatistandard.org' and parsed_url.path == '':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing this out with editing my /etc/hosts, and the parsed_url.path isn't blank:

web_1       | DOMAIN: reference.iatistandard.org
web_1       | PATH: /

Also to cover our bases, I think we should include the /en/ and /fr/ paths as some people may have cached redirects to those. And then lastly I wouldn't hardcode the /en/ in the redirect, and just redirect it to /iati-standard/. The locale redirects will direct the user to the correct /en/iati-standard or /fr/iati-standard based on their browser. So all together:

        if domain == 'reference.iatistandard.org' and parsed_url.path in ['/', '/en/', '/fr/']:
            return http.HttpResponsePermanentRedirect(f'{settings.REFERENCE_REDIRECT_BASE_URL}/iati-standard/')

@ThatcherK ThatcherK requested a review from akmiller01 May 8, 2023 07:36
@ThatcherK ThatcherK merged commit 0b6ca2c into develop May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants